Skip search shards with INDEX_REFRESH_BLOCK #129132

benchaplin · 2025-06-09T05:56:43Z

#117543 introduced a ClusterBlock which is applied to new indices in Serverless which do not yet have search shards up. We should skip searches for indices with this block in order to avoid meaningless 503s.

Edit: I've changed approaches to this "skipping" logic and wanted to document some reasoning:

I considered two options:

Hold the same search shard iterators, but mark them as "skipped"
Skip the index before resolving search shard iterators

Option 1 would result in a search response with {"_shards": { "skipped": = the number of shards we skipped due to an index refresh block. Option 2 pretends those shards don't exist, so {"_shards": { "skipped": 0, "total": < total in option 1.

I've decided to go with option 2. The reason is that this cluster block is only active when the index was just created and is still in a red state. I could not think of a reason that a user would need to be aware of the number of skipped shards in this case (a user curious about this detail would likely be able to deduce what's going on anyway). Furthermore, it simplifies the code change (see 86c9a5d).

elasticsearchmachine · 2025-06-09T05:57:12Z

Pinging @elastic/es-search-foundations (Team:Search Foundations)

tlrx

Left some comments. The search part should be reviewed by the ES Search team.

server/src/main/java/org/elasticsearch/cluster/metadata/IndexMetadata.java

tlrx · 2025-06-12T09:13:29Z

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

+        var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test");
+        client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();


The refresh block should be added automatically to newly created indices as long as they have replicas and the "use refresh block" setting is enabled in the node setting. We should remove the ability to add the refresh block through the Add Index Block API.

Thanks for taking a look @tlrx!

I was hoping to test this change outside of the context of Serverless. But I agree it's not appropriate to add the refresh block to that API for testing purposes only, so I will see if I can construct the scenario in some other way.

Alright, I was able to get the setup I was looking for by adding the block directly to cluster state in the tests.

tlrx · 2025-06-12T09:16:17Z

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

+        assertHitCount(prepareSearch().setQuery(QueryBuilders.matchAllQuery()), 0);
+    }
+
+    public void testSearchMultipleIndicesEachWithAnIndexRefreshBlock() {


I think this could be folded into a single test, where one or more indices are randomly created, most of some with replicas but other without replicas, and then allocate zero or more search shards and check the expected results, finally assigning all search shards and check the results again.

I've folded this into a single test with some additional randomization. My goal is to keep the integration tests in the Serverless PR, so I'll add the test scenario you're proposing there.

cbuescher

I did a first pass on the search related side of things and left a few questions and comments.

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java

…luster state in tests

tlrx

LGTM but I know nothing about search shard iterators :)

tlrx · 2025-07-07T09:18:47Z

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

+            ClusterService clusterService = internalCluster().getInstance(ClusterService.class, dataNode.getName());
+            ClusterState currentState = clusterService.state();
+            ClusterState newState = ClusterState.builder(currentState).blocks(blocksBuilder).build();
+            setState(clusterService, newState);


This method is not intended to be used in integration test as it overrides the current data node cluster state.

For testing the INDEX_REFRESH_BLOCK I think it makes sense to only have unit tests in stateful elasticsearch.

Sorry @tlrx, can you explain more the risks of doing this?

I like having fine-grained control over blocks so I can write tests that block some indices and allow others in one search - I think this is critical to test. If I can only 'set' the block by controlling active search nodes (like I do in the other PR), I can't think of a way to achieve what I want.

server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java

server/src/main/java/org/elasticsearch/action/search/CanMatchPreFilterSearchPhase.java

server/src/main/java/org/elasticsearch/action/search/SearchShardIterator.java

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

smalyshev · 2025-07-15T18:12:51Z

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertHitCount;
+import static org.elasticsearch.test.hamcrest.ElasticsearchAssertions.assertResponse;
+
+public class SearchWithIndexBlocksIT extends ESIntegTestCase {


Do we also want to have ESQL test for this case?

I'm tracking that to be a followup task.

javanna

I left a small question, I like how small the change has become!

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

cla-checker-service · 2025-07-21T14:26:10Z

❌ Author of the following commits did not sign a Contributor Agreement:
cdb4bc1, 17706e2

Please, read and sign the above mentioned agreement if you want to contribute to this project

javanna

Left a couple more questions. LGTM though, no need to wait further.

javanna · 2025-07-21T17:31:15Z

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java


+    static String[] ignoreBlockedIndices(ProjectState projectState, String[] concreteIndices) {
+        // optimization: mostly we do not have any blocks so there's no point in the expensive per-index checking
+        boolean hasIndexBlocks = projectState.blocks().indices(projectState.projectId()).isEmpty() == false;


nit: did we have a chance to incorporate this logic in buildPerIndexOriginalIndices perhaps, where we already look at blocks?

I want to filter concreteIndices as it's used in multiple places. However, it may be possible to move the block checks in buildPerIndexOriginalIndices to ignoreBlockedIndices. I will have to verify, then if so, I'll follow up with the change.

javanna · 2025-07-21T17:32:04Z

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java

        Set<ResolvedExpression> indicesAndAliases,
        String[] concreteIndices
    ) {
+        concreteIndices = ignoreBlockedIndices(projectState, concreteIndices);


Do I understand correctly that search shards will inherit the new behaviour as it calls getLocalShardsIterator? Other API that we need to update to look at this block?

Yes, I've added a test to verify search shards respects the block.

javanna · 2025-07-21T17:32:30Z

server/src/test/java/org/elasticsearch/action/search/TransportSearchActionTests.java

        }
    }
+
+    public void testIgnoreBlockedIndices() {


Maybe specify which block?

javanna · 2025-07-21T17:33:20Z

server/src/internalClusterTest/java/org/elasticsearch/search/SearchWithIndexBlocksIT.java

+        assertHitCount(prepareSearch().setQuery(QueryBuilders.matchAllQuery()), expectedHits);
+    }
+
+    public void testOpenPITOnIndicesWithIndexRefreshBlocks() {


What was your conclusion on open PIT? We do the same filtering, correct?

Yes, we filter just the same - that's verified in this test by asserting the hit count does not include docs from blocked indices.

…king * upstream/main: (100 commits) Term vector API on stateless search nodes (elastic#129902) TEST Fix ThreadPoolMergeSchedulerStressTestIT testMergingFallsBehindAndThenCatchesUp (elastic#131636) Add inference.put_custom rest-api-spec (elastic#131660) ESQL: Fewer serverless docs in tests (elastic#131651) Skip search on indices with INDEX_REFRESH_BLOCK (elastic#129132) Mute org.elasticsearch.indices.cluster.RemoteSearchForceConnectTimeoutIT testTimeoutSetting elastic#131656 [jdk] Resolve EA OpenJDK builds to our JDK archive (elastic#131237) Add optimized path for intermediate values aggregator (elastic#131390) Correctly handling download_database_on_pipeline_creation within a pipeline processor within a default or final pipeline (elastic#131236) Refresh potential lost connections at query start for `_search` (elastic#130463) Add template_id to patterned-text type (elastic#131401) Integrate LIKE/RLIKE LIST with ReplaceStringCasingWithInsensitiveRegexMatch rule (elastic#131531) [ES|QL] Add doc for the COMPLETION command (elastic#131010) ESQL: Add times to topn status (elastic#131555) ESQL: Add asynchronous pre-optimization step for logical plan (elastic#131440) ES|QL: Improve generative tests for FORK [130015] (elastic#131206) Update index mapping update privileges (elastic#130894) ESQL: Added Sample operator NamedWritable to plugin (elastic#131541) update `kibana_system` to grant it access to `.chat-*` system index (elastic#131419) Clarify heap size configuration (elastic#131607) ...

…-tracking * upstream/main: (44 commits) Term vector API on stateless search nodes (elastic#129902) TEST Fix ThreadPoolMergeSchedulerStressTestIT testMergingFallsBehindAndThenCatchesUp (elastic#131636) Add inference.put_custom rest-api-spec (elastic#131660) ESQL: Fewer serverless docs in tests (elastic#131651) Skip search on indices with INDEX_REFRESH_BLOCK (elastic#129132) Mute org.elasticsearch.indices.cluster.RemoteSearchForceConnectTimeoutIT testTimeoutSetting elastic#131656 [jdk] Resolve EA OpenJDK builds to our JDK archive (elastic#131237) Add optimized path for intermediate values aggregator (elastic#131390) Correctly handling download_database_on_pipeline_creation within a pipeline processor within a default or final pipeline (elastic#131236) Refresh potential lost connections at query start for `_search` (elastic#130463) Add template_id to patterned-text type (elastic#131401) Integrate LIKE/RLIKE LIST with ReplaceStringCasingWithInsensitiveRegexMatch rule (elastic#131531) [ES|QL] Add doc for the COMPLETION command (elastic#131010) ESQL: Add times to topn status (elastic#131555) ESQL: Add asynchronous pre-optimization step for logical plan (elastic#131440) ES|QL: Improve generative tests for FORK [130015] (elastic#131206) Update index mapping update privileges (elastic#130894) ESQL: Added Sample operator NamedWritable to plugin (elastic#131541) update `kibana_system` to grant it access to `.chat-*` system index (elastic#131419) Clarify heap size configuration (elastic#131607) ...

benchaplin added 5 commits June 4, 2025 19:00

Skip indices that have an index refresh block

2aa74e3

Merge branch 'main' into skip_search_shards_with_index_block

12b6b81

Construct the iterator skipped

9c705cd

Fix javadocs

1c75721

Add unit test

1ecc447

benchaplin requested a review from tlrx June 9, 2025 05:56

benchaplin added >non-issue Team:Search Foundations Meta label for the Search Foundations team in Elasticsearch :Search Foundations/Search Catch all for Search Foundations v9.1.0 labels Jun 9, 2025

elasticsearchmachine added the serverless-linked Added by automation, don't add manually label Jun 9, 2025

benchaplin requested a review from a team as a code owner June 9, 2025 14:39

benchaplin force-pushed the skip_search_shards_with_index_block branch from 998cdd5 to 1ecc447 Compare June 9, 2025 17:05

benchaplin removed the request for review from a team June 9, 2025 17:06

elasticsearchmachine and others added 9 commits June 9, 2025 17:13

[CI] Auto commit changes from spotless

cdb4bc1

Merge branch 'main' into skip_search_shards_with_index_block

b7ade2d

Merge branch 'main' into skip_search_shards_with_index_block

cd991c2

Merge branch 'main' into skip_search_shards_with_index_block

5f50d5c

Rewrite DFS if processing one or zero unskipped shard iterators

3f86fb8

Make can-match support already skipped shard iterators

0edc27c

Add IT for executing search and PIT against refresh blocked indices

9de6f06

Fix resource leak by using decRef assertion

be37bf6

[CI] Auto commit changes from spotless

17706e2

benchaplin requested review from a team as code owners June 11, 2025 21:03

benchaplin force-pushed the skip_search_shards_with_index_block branch from e233cc7 to 17706e2 Compare June 11, 2025 21:04

Merge branch 'main' into skip_search_shards_with_index_block

8759a07

tlrx reviewed Jun 12, 2025

View reviewed changes

cbuescher reviewed Jun 18, 2025

View reviewed changes

benchaplin added 5 commits July 1, 2025 12:35

Remove constructor used only in tests

0f0200a

Fix missed merge conflict

7689263

Remove ability to set INDEX_REFRESH_BLOCK from API, add directly to c…

598e906

…luster state in tests

Merge branch 'main' into skip_search_shards_with_index_block

4cdfbd0

Merge branch 'main' into skip_search_shards_with_index_block

76ecade

tlrx reviewed Jul 7, 2025

View reviewed changes

javanna reviewed Jul 10, 2025

View reviewed changes

benchaplin added 4 commits July 15, 2025 11:03

Rework change to ignore blocked indices before shard resolution

86c9a5d

Merge branch 'main' into skip_search_shards_with_index_block

7200edc

Clean up

d72600b

Add _msearch test case

61d40c4

smalyshev reviewed Jul 15, 2025

View reviewed changes

rudolf mentioned this pull request Jul 16, 2025

[docs] ES tutorial on index initialization elastic/kibana#224628

Merged

9 tasks

benchaplin requested a review from javanna July 17, 2025 15:15

javanna reviewed Jul 18, 2025

View reviewed changes

server/src/main/java/org/elasticsearch/action/search/TransportSearchAction.java Outdated Show resolved Hide resolved

Revert search type rewrite change

51d5196

benchaplin removed request for a team July 18, 2025 14:09

benchaplin added 2 commits July 18, 2025 11:18

Merge branch 'main' into skip_search_shards_with_index_block

24f2770

Merge branch 'main' into skip_search_shards_with_index_block

b33a14f

javanna approved these changes Jul 21, 2025

View reviewed changes

benchaplin added 2 commits July 21, 2025 15:15

Add search shards test

e10d59b

Oops remove Repeat

31d7dce

benchaplin merged commit cc7bbe4 into elastic:main Jul 21, 2025
32 of 33 checks passed

rudolf mentioned this pull request Jul 23, 2025

[migrations] ZDT Wait for yellow source if index already exists elastic/kibana#224694

Closed

benchaplin mentioned this pull request Jul 31, 2025

Skip indices with INDEX_REFRESH_BLOCK in field caps #132291

Closed

benchaplin mentioned this pull request Oct 1, 2025

Enable the index refresh block #135785

Open

		var addIndexBlockRequest = new AddIndexBlockRequest(IndexMetadata.APIBlock.REFRESH, "test");
		client().execute(TransportAddIndexBlockAction.TYPE, addIndexBlockRequest).actionGet();

Uh oh!

Skip search shards with INDEX_REFRESH_BLOCK #129132

Skip search shards with INDEX_REFRESH_BLOCK #129132

Uh oh!

Conversation

benchaplin commented Jun 9, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Jun 9, 2025

Uh oh!

tlrx left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

cbuescher left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

tlrx left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

cla-checker-service bot commented Jul 21, 2025

Uh oh!

javanna left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

benchaplin commented Jun 9, 2025 •

edited

Loading